Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Coverage Reporting for Gremlin JS #24

Open
wants to merge 2 commits into
base: 3.5-dev
Choose a base branch
from

Conversation

L0Lmaker
Copy link

No description provided.

@L0Lmaker L0Lmaker force-pushed the rithin/an-1208-gremlin-js-coverage branch from b20deda to 135a30a Compare August 22, 2022 23:46
@L0Lmaker L0Lmaker marked this pull request as ready for review August 22, 2022 23:47
"TODO": "# test other mime types like graphbinary stringd",
"features": "npm run features-graphson30 && npm run features-graphbinary",
"features-graphson30": "cross-env CLIENT_MIMETYPE='application/vnd.gremlin-v3.0+json' cucumber-js --require test/cucumber ../../../../../gremlin-test/features/",
"features-graphbinary": "cross-env CLIENT_MIMETYPE='application/vnd.graphbinary-v1.0' cucumber-js --require test/cucumber ../../../../../gremlin-test/features/",
"features-docker": "npm run features-graphson30-docker && npm run features-graphbinary-docker",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't these the commands for running this in Docker?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, these are unnecessary, will remove them

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed in 2c56b8a

@codecov
Copy link

codecov bot commented Aug 23, 2022

Codecov Report

Merging #24 (2c56b8a) into 3.5-dev (cda6788) will increase coverage by 7.86%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           3.5-dev      #24      +/-   ##
===========================================
+ Coverage    63.58%   71.45%   +7.86%     
===========================================
  Files           23       81      +58     
  Lines         3636     6656    +3020     
===========================================
+ Hits          2312     4756    +2444     
- Misses        1145     1721     +576     
  Partials       179      179              
Impacted Files Coverage Δ
.../structure/io/binary/internals/StringSerializer.js 100.00% <0.00%> (ø)
...tructure/io/binary/internals/BytecodeSerializer.js 72.44% <0.00%> (ø)
...script/gremlin-javascript/lib/process/traversal.js 78.68% <0.00%> (ø)
.../structure/io/binary/internals/DoubleSerializer.js 100.00% <0.00%> (ø)
...pt/gremlin-javascript/lib/driver/response-error.js 100.00% <0.00%> (ø)
...ascript/lib/structure/io/binary/internals/utils.js 94.73% <0.00%> (ø)
...in-javascript/lib/structure/io/graph-serializer.js 79.51% <0.00%> (ø)
...in-javascript/lib/structure/io/type-serializers.js 77.69% <0.00%> (ø)
...ib/structure/io/binary/internals/LongSerializer.js 100.00% <0.00%> (ø)
...mlin-javascript/lib/process/anonymous-traversal.js 100.00% <0.00%> (ø)
... and 48 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@L0Lmaker L0Lmaker requested a review from xiazcy August 23, 2022 14:52
@@ -4,6 +4,16 @@
"lockfileVersion": 1,
"requires": true,
"dependencies": {
"@ampproject/remapping": {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering why the package-lock file is adding a large chunk of code again, is it the node version?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

installed with same node and npm version, the changes refer to the dependencies and sub-dependencies of the nyc dependency that is added in this PR

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added 100 dependencies, but it's ok for node world %)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants